Conversation
# Conflicts: # src/Logs/LogsAggregator.php # src/Metrics/MetricsAggregator.php # src/SentrySdk.php # src/Tracing/Span.php # src/functions.php # tests/FunctionsTest.php # tests/Logs/LogsAggregatorTest.php # tests/Monolog/LogsHandlerTest.php # tests/State/HubTest.php
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| if ($scope->propagationContext !== null && $scope->getType() !== ScopeType::current()) { | ||
| $this->propagationContext = $scope->propagationContext; | ||
| } |
There was a problem hiding this comment.
mergeFrom doesn't clone propagationContext unlike __clone does
Low Severity
mergeFrom assigns propagationContext by direct reference instead of cloning it, unlike __clone which deep-clones both user and propagationContext. This creates shared mutable state between the merged scope and the source (isolation) scope. Since PropagationContext has mutable setters and a lazy-initializing toBaggage() method, any mutation through the merged scope would unexpectedly affect the isolation scope's propagation context. The user field in mergeFrom is correctly cloned, making this inconsistency easy to overlook.
Additional Locations (1)
| if ($user->getEmail() !== null && !isset($scopeAttributes['user.email'])) { | ||
| $scopeAttributes['user.email'] = $user->getEmail(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Inconsistent user/scope-attribute precedence between logs and metrics
Medium Severity
In MetricsAggregator, user.id/user.email/user.name from getUser() are only added if the corresponding scope attribute does NOT already exist (!isset($scopeAttributes['user.id'])). In LogsAggregator, user data is set unconditionally after applyScopeAttributes, always overriding scope attributes with the same keys. This means scope attributes take precedence over user data for metrics but not for logs — an inconsistency introduced by this PR (the old code had no scope attribute checks for either).


This PR replaces the Hub based approach to the new scope based approach. The main difference is that instead of having one Hub with multiple Scope layers, data is now divided into different scopes, namely GlobalScope, IsolationScope and CurrentScope. Further details are documented here: https://develop.sentry.dev/sdk/telemetry/scopes/